Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Create proposal for annotation-based filtering #1797

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

asafalgawi
Copy link

@asafalgawi asafalgawi commented Sep 12, 2024

Description

What this PR does / why we need it:

This PR adds a proposal for filtering artifacts based on their annotations before forwarding them to verification by one of the configured verifiers.]

The PR refers to issue #1772

Type of change

Added document to proposal directory.

@asafalgawi asafalgawi changed the title Create proposal for annotation-based filtering docs: Create proposal for annotation-based filtering Sep 12, 2024
spec:
name: # REQUIRED: [string], the unique type of the verifier (notation, cosign)
artifactType: # REQUIRED: [string], comma seperated list, artifact type this verifier handles
artifactAgeFilter: # Optional: [string], one of the following 'latest' to verify only the last artifact, or all to verify all artifacts. defaults to all.
Copy link
Collaborator

@susanshi susanshi Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if artifactAgeFilter may contain different values based on verifier, should this be moved to "parameters"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added it here because each verifier/namespaced verifier configuration is evaluated as a different "verifier" by ratify.

Since the properties part is parsed by the verifier code itself, and as you said, we wish to filter in the executor, then it would be better to place those fields outside the scope of parameters.

spec:
name: # REQUIRED: [string], the unique type of the verifier (notation, cosign)
artifactType: # REQUIRED: [string], comma seperated list, artifact type this verifier handles
artifactAgeFilter: # Optional: [string], one of the following 'latest' to verify only the last artifact, or all to verify all artifacts. defaults to all.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the value of artifactAgeFilter be a define set of string? an expression or key:value pair

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be as a single string, either "latest" or "all"


### Defining artifact age based filtering

To support artifact age based filtering, we would add an additional field to the verifier configuration:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the filtering capability be available for both k8s and cli scenario.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it should be available on both methods, the examples for K8s are perhaps just out of lazyness :)
I'll update the proposal to include examples of CLI configurations.

@susanshi
Copy link
Collaborator

Hi @asafalgawi , thank you for the proposal. We have discussed this during the community meeting. We agree with the "latest" vulnerability report user scenario, however we need to understand the feasibility and scope of change. @binbin-li brought up good point today, the individual verifier might not have filtering abilities. Major refactoring might be required in executor code path. We still require more time to understand the notary scenarios a bit more, thankyou for your patience.

@asafalgawi
Copy link
Author

Hi @susanshi, thanks for the reviewing the proposal and for the update :)

Copy link
Collaborator

@yizha1 yizha1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @asafalgawi . I left some comments.

## Problem/Motivation

When configuring a verifier in Ratify, we set the artifact type the verifier should work on. In such case, Ratify will verify all referrers of a given subject that have a matching artifact type using the verifier.
In some cases, this could lead to a wrong behavior. For instance, Vulnerability Artifacts are outdated once a new artifact is written to the repository, as such there is no use for verifying both the new one and the old one. On the other hand, in performing signature verification using Notary, we may wish to attest more than one signature, because there may be multiple signatures attached to a given artifact, but our "tenant" only trusts a subset of them.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In some cases, this could lead to a wrong behavior. For instance, Vulnerability Artifacts are outdated once a new artifact is written to the repository, as such there is no use for verifying both the new one and the old one. On the other hand, in performing signature verification using Notary, we may wish to attest more than one signature, because there may be multiple signatures attached to a given artifact, but our "tenant" only trusts a subset of them.
In some cases, this could lead to a wrong behavior. For instance, Vulnerability Artifacts are outdated once a new artifact is written to the repository, as such there is no use for verifying both the new one and the old one. On the other hand, in performing Notary Project signature verification, we may wish to attest more than one signature, because there may be multiple signatures attached to a given artifact, but our "tenant" only trusts a subset of them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current behavior of Notary Project signature verification is:

  • if 1 out of N signatures passes verification, then the overall signature verification succeeds.
  • If all signatures failed validation, then the overall signature verification fails

Copy link
Collaborator

@yizha1 yizha1 Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@binbin-li If I remembered correctly, verifying multiple signatures can be achieved through Rego policies?

"digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b864",
"signatures": [
{
"mediaType": "applicaiton/vnd.cncf.notary.x509-signature.config.v2",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this media type applicaiton/vnd.cncf.notary.x509-signature.config.v2 correct?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied it from their website, perhaps it is not up to date, i'll check again and update the doc.


# Filtration Methods

## Age Based Filtration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still annotation-based filtration, and it just uses the annotation org.opencontainers.image.created to achieve the goal. So I think there is only one filtration method, but currently there are two scenarios, one is for filtering out latest artifact, another is for filtering out a subset of artifacts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but the verification process of both is different.
Age based verification requires context, you need to be aware of all the referrers, while annotation based in specific to the artifact that is currently being verified.

While age based has to be implemented in the executor, the annotation filtering can be done even in the skel method of the verifier, it could return a new type of response to show that verification was skipped and the artifact won't be included in the final verification response.

Perhaps we should not bundle both features in the same proposal ?

```

We could define a notation verifier configuration that would require the artifact manifest to contain the annotation `org.opencontainers.image.signature.fingerprint` and that is should have a known fingerprint value. this would enable a scenario where the user configures three notation verifiers, one in each namespace:
* Namepsace A -> Fingerprint should match [X]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate more about the problem that fingerprint filtering is going to be solved? If it is to distinguish different signers, we can configure Notation Verifier with different trusted identities for different namespaces. For example, images pulled in namespace A are signed by Signer-A, then we can configure the trusted identities that Signer-A used for namespace A.

Or is your scenario to distinguish signatures that are generated by the same Signer (trusted identity)? Then what is the difference in signatures generated by the same Signer? If it is about age, maybe we can use aged-filtering to verify only the latest signature?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Basically, I referred to a scenario in which I as a namespace owner I require that all certificate chains will contains a specific signer, since the signature chain is part of the artifact annotations, I can verify it before even going through all whole process of verifying the actual signature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants